-
Notifications
You must be signed in to change notification settings - Fork 586
refactor: improve ranked choice voting readability and add test #1177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refactors the ranked choice voting implementation to improve code readability through better documentation and function naming. It also adds comprehensive test coverage for edge cases and the newly exported getFinalRound
function.
Key changes:
- Improved function naming and added extensive JSDoc documentation for better API clarity
- Exported
getFinalRound
function and added comprehensive test coverage for IRV algorithm edge cases - Enhanced
getChoiceString
method to properly handle invalid choice indices using filter instead of conditional mapping
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/voting/rankedChoice.ts | Adds comprehensive JSDoc documentation, renames functions for clarity, exports getFinalRound, and improves getChoiceString implementation |
src/voting/rankedChoice.spec.js | Adds extensive test coverage for getFinalRound function, isValidChoice validation, and edge cases including invalid choice indices |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
* // Candidate 2 wins with 150 total voting power (60+50+40 from 3 strategies) | ||
* // Candidate 1 has 120 total voting power (70+30+20 from 3 strategies) | ||
*/ | ||
export function getFinalRound( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exporting getFinalRound creates a breaking change to the module's public API. Consider whether this function should be exported or if it's intended for internal use only.
export function getFinalRound( | |
function getFinalRound( |
Copilot uses AI. Check for mistakes.
Not an important PR, don't need a release after merge, can wait next fix/feature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tAck
This PR improves comprehension of the ranked choice voting system by adding more documentation, and add more test.
Add more docs
Add more docs to some function, to help comprehension, and understand returned value.
Add more tests
Add more tests, justifying that the fix #1176 is really needed, and not just an edge case.
This fix is not related to invalid votes, etc..., the scenario can happen if the irv function pass on the first round, and the first round do no contain all the votes.
Adjust
getChoiceString
Update function to handle invalid choice: they're now ignored, instead of showing
undefined
, like on the approval/quadratic/weighted voting system.This is consistent with the score computation, where those invalid votes are already ignored